Skip to content

Updater safety + config/CLI robustness (B13, B18, B20, B22) - #372

Merged
andrewklingelhofer merged 3 commits into
mainfrom
fix/audit-updater-config-cli
Jul 18, 2026
Merged

Updater safety + config/CLI robustness (B13, B18, B20, B22)#372
andrewklingelhofer merged 3 commits into
mainfrom
fix/audit-updater-config-cli

Conversation

@andrewklingelhofer

Copy link
Copy Markdown
Contributor

Summary

Self-update integrity and a cluster of smaller config/CLI/display bugs.

# Problem Fix
B13 Auto-updater restarted even when the update failed → futile restart loop on an unchanging version checkForUpdate returns success; gate restart on it; defer when work is queued
+ Updater could install a truncated binary / one with no checksum Reject length != totalBytes; treat a missing checksum as a hard failure
B18 Empty/comment-only config crashed loadConfig with a cryptic TypeError parseYaml(raw) ?? {} in all three readers; drop redundant re-validation
B20 StatusContext (legacy commit statuses) read as perpetually pending; skipped/neutral checks too Normalize verdict from conclusion ?? state; non-failing terminal conclusions → success; add a gh pr view timeout; batch instead of dropping past the cap
B22 logs --follow printed literal \x1b[36m instead of color Interpolate a real ESC byte
+ loadEnvFallback didn't strip quotes/export Bearer "lin_…" 401s Strip surrounding quotes + leading export

Tests

audit-updater-misc.test.ts — empty config returns {}; SKIPPED/NEUTRAL/STALE/CANCELLED → success while TIMED_OUT/ACTION_REQUIRED → failure; truncated download rejected (valid-checksum isolation); env quote stripping.

Verification

typecheck clean · lint exit 0 · bun test 859 pass / 0 fail.

🤖 Generated with Claude Code

andrewklingelhofer and others added 3 commits June 15, 2026 21:54
B13: auto-updater no longer restarts on a failed update. checkForUpdate
now returns a boolean (true only when the renameSync over process.execPath
succeeds); tick() gates restartFn() on it and closes out the Slack
"auto-updating" message on failure. Auto-update is also deferred when
getQueueSize() > 0, mirroring cleanupStale.

Updater integrity: reject truncated downloads (buffer.length !==
Content-Length) and treat a missing checksum asset as a hard failure for
auto-update (new requireChecksum opt, set by the auto-updater) so we never
rename in an unverified binary. Manual `update` behavior is unchanged.

B18: empty/comment-only config no longer throws a cryptic TypeError —
parseYaml(raw) ?? {} in loadConfig, loadWorkDir, and loadCleanConfig
(mirroring validate.ts). Dropped the redundant validateCritterType loop
(parseCritterType already validates).

B20: pr-status normalizes statusCheckRollup per-entry via conclusion ??
state, so legacy StatusContext nodes map to success/failure instead of
perpetual pending. Added a 10s timeout to the gh pr view call (via
runCommand timeoutMs, which also drains stderr) and process toFetch in
sequential batches instead of dropping work beyond the cap (F6).

B22: logs --follow interpolates a real ESC byte instead of literal
"\x1b[36m". De-duped extractTimestamp/newestDir and the regex-escape
helper by exporting them from log-resolver.ts.

env.ts: loadEnvFallback strips a surrounding quote pair and supports a
leading `export ` so quoted tokens no longer leak quotes into values.

utils.ts: runCommand gains an optional timeoutMs (default unchanged).

Tests: src/__tests__/audit-updater-misc.test.ts covers B18, B20, env quote
stripping, and checkForUpdate returning false on truncated/checksum-failed/
missing-checksum downloads.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
…ests

normalizeCheckVerdict treated a COMPLETED CheckRun with conclusion
SKIPPED/NEUTRAL/STALE/CANCELLED as 'pending' (hourglass forever), where
the original code read it as success. Classify all recognized non-failing
terminal conclusions as success, keep only in-flight/unknown states pending,
and keep FAILURE/ERROR/TIMED_OUT/ACTION_REQUIRED (and STARTUP_FAILURE) as
failure. StatusContext (state-based) handling from B20 is preserved.

Tests: add coverage for SKIPPED/NEUTRAL/STALE/CANCELLED → success, and make
the truncated-download test use a valid checksum so it isolates the
truncation guard instead of leaning on a bad checksum.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
resolvePhaseMcpConfig expands ~ via node:os homedir(); the test compared it to
process.env.HOME. Bun caches homedir() on first call and ignores later
process.env.HOME mutations, so when a sibling test changes HOME they diverge on
Linux CI and this test fails (works on macOS by execution-order luck). Assert
against the same homedir() the implementation uses.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
@andrewklingelhofer
andrewklingelhofer merged commit f640036 into main Jul 18, 2026
1 check passed
@andrewklingelhofer
andrewklingelhofer deleted the fix/audit-updater-config-cli branch July 18, 2026 17:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant